Skip to content

re-export async-attributes#238

Merged
5 commits merged into
masterfrom
attributes
Nov 7, 2019
Merged

re-export async-attributes#238
5 commits merged into
masterfrom
attributes

Conversation

@yoshuawuyts

Copy link
Copy Markdown
Contributor

Closes #237. The docs don't render properly on this, but I think that's okay. Thanks!

Signed-off-by: Yoshua Wuyts <yoshuawuyts@gmail.com>
Signed-off-by: Yoshua Wuyts <yoshuawuyts@gmail.com>
Signed-off-by: Yoshua Wuyts <yoshuawuyts@gmail.com>
@yoshuawuyts

yoshuawuyts commented Nov 2, 2019

Copy link
Copy Markdown
Contributor Author

This PR and async-rs/async-attributes#8 have been updated to now use the attributes feature as suggested in #438.

I think merging this would actually be really nice, as we enable people to write some nicer-looking applications the way they could with Runtime, but we don't lose any performance in the base case. Coupled with the runtime feature I think async-std will be a really well rounded option for almost every use!

Screenshots

Screenshot_2019-11-03 async_std - Rust

@ghost

ghost commented Nov 2, 2019

Copy link
Copy Markdown

Are these now stable attributes? I assume so and fine with that, but just making sure it's not an oversight. :)

There's not much we can do wrong with these attributes - they're pretty straightforward, right?

@ghost ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving, but let's wait for @skade to also take a look.

@ghost ghost requested a review from skade November 2, 2019 23:37
@yoshuawuyts

yoshuawuyts commented Nov 2, 2019

Copy link
Copy Markdown
Contributor Author

There's not much we can do wrong with these attributes - they're pretty straightforward, right?

Not necessarily, and I think we should be cautious. async-rs/async-attributes#11 and async-rs/async-attributes#9 actually should be resolved first.

edit: I don't think we should be exposing that particular API. So another option is we punt #[bench] until we figure it out and only expose #[test] and #[main]. Those two could be insta-stable for sure.

@yoshuawuyts

Copy link
Copy Markdown
Contributor Author

Updated the PR to be exactly that. Thanks!

Signed-off-by: Yoshua Wuyts <yoshuawuyts@gmail.com>
Signed-off-by: Yoshua Wuyts <yoshuawuyts@gmail.com>
@ghost

ghost commented Nov 2, 2019

Copy link
Copy Markdown

Agree with everything you said 👍

@ghost

ghost commented Nov 6, 2019

Copy link
Copy Markdown

ping for @skade

@yoshuawuyts

yoshuawuyts commented Nov 7, 2019

Copy link
Copy Markdown
Contributor Author

@stjepang I would like to make this available in the next release; I'd like to propose we merge this.

@ghost ghost merged commit 84880c4 into master Nov 7, 2019
@ghost ghost deleted the attributes branch November 7, 2019 22:10
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose async-attributes macros behind feature flag

1 participant